Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Scaladoc markdown preprocessor #13140

Merged
merged 6 commits into from
Oct 19, 2021

Conversation

BarkingBad
Copy link
Contributor

PoC of preprocessing markdown to get advantage of scaladoc linking and snippet compiler but preserve the flexibility to reuse docs in Jekyll or another templating engine.

@BarkingBad BarkingBad changed the title Markdown preprocessor Scaladoc markdown preprocessor Jul 23, 2021
@BarkingBad BarkingBad force-pushed the scaladoc/markdowns-preprocessor branch from 2627f99 to 8ec44e0 Compare August 11, 2021 18:15
@BarkingBad BarkingBad marked this pull request as ready for review August 12, 2021 12:46
@BarkingBad BarkingBad force-pushed the scaladoc/markdowns-preprocessor branch from db71769 to 8c43294 Compare August 12, 2021 12:47
@BarkingBad BarkingBad force-pushed the scaladoc/markdowns-preprocessor branch from 51698d2 to bb13556 Compare August 12, 2021 14:13
Copy link
Contributor

@julienrf julienrf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should not try to support both rendering the dotty website with scaladoc, and processing the md files to be rendered by Jekyll (on docs.scala-lang.org).

Instead, we should just focus on doing one thing: processing the md files that will be then rendered by Jekyll only. This will save us from introducing the postprocessing-layout feature.

Reviewing the changes made to the md files is hard, I suggest the following process to be sure they are correct:

  1. update the markdown files in the docs.scala-lang repo to contain the fixes that have been applied in the dotty repo (the files in docs.scala-lang have been taken from commit 270bd8b, so we need to see what changes have been made to the docs/ files since that commit and to backport them to the docs.scala-lang repo).
  2. copy the files from docs.scala-lang to the dotty repo, without changing anything to them

docs/docs/reference/contextual/context-bounds.md Outdated Show resolved Hide resolved
docs/docs/reference/contextual/extension-methods.md Outdated Show resolved Hide resolved
docs/docs/reference/other-new-features/trait-parameters.md Outdated Show resolved Hide resolved
docs/docs/usage/getting-started.md Outdated Show resolved Hide resolved
project/scripts/genDocsScalaLang Outdated Show resolved Hide resolved
project/scripts/genDocsScalaLang Show resolved Hide resolved
scaladoc-js/resources/code-snippets.css Outdated Show resolved Hide resolved
scaladoc-js/src/Main.scala Show resolved Hide resolved
@BarkingBad
Copy link
Contributor Author

Thank you Julien for your patience to review this big chunk of code :D I will address all of your concerns in the threads.

@BarkingBad
Copy link
Contributor Author

BarkingBad commented Aug 18, 2021

I think we should not try to support both rendering the dotty website with scaladoc, and processing the md files to be rendered by Jekyll (on docs.scala-lang.org).

Instead, we should just focus on doing one thing: processing the md files that will be then rendered by Jekyll only. This will save us from introducing the postprocessing-layout feature.

I agree

Reviewing the changes made to the md files is hard, I suggest the following process to be sure they are correct:

  1. update the markdown files in the docs.scala-lang repo to contain the fixes that have been applied in the dotty repo (the files in docs.scala-lang have been taken from commit 270bd8b, so we need to see what changes have been made to the docs/ files since that commit and to backport them to the docs.scala-lang repo).
  2. copy the files from docs.scala-lang to the dotty repo, without changing anything to them

I was thinking about adding one more feature to the markdown front-matters, like file source. We can easily generate it because we know the source-links from scaladoc. These links would be used as a button target for Edit on github. From the files perspective they won't change, as this entry in the front-matter would be autogenerated for dotty markdown files by scaladoc. I wonder, whether we could add something similar for the markdowns hosted at docs.scala-lang to stay concise and help users introduce changes faster (no overhead of finding correct file and forking the repo)?

@julienrf
Copy link
Contributor

I wonder, whether we could add something similar for the markdowns hosted at docs.scala-lang to stay concise and help users introduce changes faster (no overhead of finding correct file and forking the repo)?

Yes, that’s a good point. We should find a way to make this work.

@BarkingBad
Copy link
Contributor Author

I'm thinking about opening 2 smaller PRs (one with css-fixes and one with markdown update) then would rebase this PR on top of the changes to reduce LOC of this PR and ease the whole process

@BarkingBad
Copy link
Contributor Author

CSSes PR: #13335

@BarkingBad
Copy link
Contributor Author

Markdown backports to the docs.scala-lang repo: scala/docs.scala-lang#2160

@BarkingBad
Copy link
Contributor Author

Moved markdowns back to dotty: #13350

@BarkingBad BarkingBad force-pushed the scaladoc/markdowns-preprocessor branch from 096472d to e3dfc31 Compare September 3, 2021 13:18
@BarkingBad BarkingBad marked this pull request as draft September 3, 2021 13:40
@BarkingBad BarkingBad force-pushed the scaladoc/markdowns-preprocessor branch 5 times, most recently from 66ce536 to a3f7e1f Compare September 29, 2021 14:02
@BarkingBad BarkingBad force-pushed the scaladoc/markdowns-preprocessor branch from a3f7e1f to d453b0e Compare September 30, 2021 13:24
@BarkingBad BarkingBad marked this pull request as ready for review October 4, 2021 11:36
Copy link
Contributor

@julienrf julienrf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did another pass where I added a few more comments. Please also note that former comments have not been addressed yet.

.github/workflows/ci.yaml Show resolved Hide resolved
.github/workflows/ci.yaml Show resolved Hide resolved
.github/workflows/ci.yaml Show resolved Hide resolved
project/scripts/genDocsScalaLang Outdated Show resolved Hide resolved
@@ -122,6 +122,16 @@ class ScaladocSettings extends SettingGroup with AllScalaSettings:

val scastieConfiguration: Setting[String] =
StringSetting("-scastie-configuration", "Scastie configuration", "Additional configuration passed to Scastie in code snippets", "")

val projectFormat: Setting[String] =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we use an enumeration type?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is already ChoiceSetting. It is hard to extends Setting to be compatible with my custom type as I would have to modify doSet method from dotty.tools.dotc.config.Settings for my new enumeration ClassTag

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. This is out of the scope of this PR, but we might need to allocate some resources at some point to clean the codebase. It looks very fragile now. If we add another format to the list we will have to manually look for all the places we pattern match on a format to check that we correctly handle it. If it was an enumeration the compiler would just give us exhaustivity warnings.

@BarkingBad BarkingBad force-pushed the scaladoc/markdowns-preprocessor branch from 942362c to 44fe92b Compare October 14, 2021 13:50
Copy link
Contributor

@julienrf julienrf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is urgent to move forward and clean the documentation authoring workflow. As long as we duplicate the markdown sources here and on docs.scala-lang, divergence will happen (see where we are in just a couple of months).

Hopefully, this work will help us maintain the documentation source files so that the content at docs.scala-lang.org will be easier to keep up to date, even for the compiler team.

By the way, I see that this PR changes some things on the source files that will be published to dotty.epfl.ch. I am not able to review those changes. I leave them up to you.

@BarkingBad BarkingBad merged commit 3f97a87 into scala:master Oct 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants